Skip to content

fix(config): Add sample data visibility env flag#778

Merged
1 commit merged intoKitware:mainfrom
arhowe00:759-Fix-seeing-Sample-Data-Browser-ever-if-configured-to-visiblity-false
Oct 7, 2025
Merged

fix(config): Add sample data visibility env flag#778
1 commit merged intoKitware:mainfrom
arhowe00:759-Fix-seeing-Sample-Data-Browser-ever-if-configured-to-visiblity-false

Conversation

@arhowe00
Copy link
Copy Markdown
Collaborator

Closes #759

Configures the initial visibility state at build time. Can prevent a benign race condition between the default (true) and runtime-fetched (false) settings where delayed state update looks ugly.

Configures the initial visibility state at build time. Can prevent a
benign race condition between the default (true) and runtime-fetched
(false) settings where delayed state update looks ugly.
@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 19, 2025

Deploy Preview for volview-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e3bce7e
🔍 Latest deploy log https://app.netlify.com/projects/volview-dev/deploys/68a4a838f4975900089c7c49
😎 Deploy Preview https://deploy-preview-778--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@PaulHax
Copy link
Copy Markdown
Collaborator

PaulHax commented Aug 19, 2025

lets remove the non working configJson.ts code that trys to configure this

@arhowe00
Copy link
Copy Markdown
Collaborator Author

lets remove the non working configJson.ts code that trys to configure this

I thought we still wanted the ability to override the .env value with the json configuration file. Do you want the configuration file to still work to enable/disable it?

@PaulHax
Copy link
Copy Markdown
Collaborator

PaulHax commented Aug 20, 2025

I thought we still wanted the ability to override the .env value with the json configuration file.

I guess I missed that requirment. I don't think that is needed. 2 places to config the same thing? There is an opprotunity to have a negative line count commit, which makes me happy.

@floryst
Copy link
Copy Markdown
Contributor

floryst commented Aug 20, 2025

I can see the argument for removing the configJson code that handles sample data visibility. Users likely won't care to configure it at runtime, and instead only care about having an app that either shows it (unlikely) or doesn't show it (likely).

Scratch the overriding idea. Let's just remove it entirely from the configJson code.

@floryst
Copy link
Copy Markdown
Contributor

floryst commented Aug 20, 2025

You'll need to add VITE_SHOW_SAMPLE_DATA to https://github.com/Kitware/VolView/blob/main/.github/workflows/checks.yml in order for the tests to pass.

@PaulHax
Copy link
Copy Markdown
Collaborator

PaulHax commented Oct 7, 2025

also need to add that env var to the netlify.tomal file

[build.environment]
  VITE_SHOW_SAMPLE_DATA= "true"

@github-merge-queue github-merge-queue Bot closed this pull request by merging all changes into Kitware:main in 7f6def4 Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix seeing "Sample Data Browser" ever if configured to visiblity false

3 participants